Skip to content

chore(lint): batch 4 of #12146 — shared/components react-hooks violations resolved - #12159

Merged
diegosouzapw merged 3 commits into
release/v3.8.51from
chore/12146-batch4-shared-components
Aug 31, 2026
Merged

diegosouzapw merged 3 commits into
release/v3.8.51from
chore/12146-batch4-shared-components

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Batch 4 of #12146: the 21 react-hooks/* React Compiler violations in the 11 src/shared/components files are resolved with real refactors — no eslint-disable, and every react-hooks/* suppression entry for these files is removed from config/quality/eslint-suppressions.json (entries for other rules are preserved).

Violations resolved

Rule Count Technique
set-state-in-effect 16 (a) prop/state mirrors and modal open/close resets → guarded render-time adjustment (react.dev "You Might Not Need an Effect" prev-tracking); (b) fetch+set effects → async loader moved inside the effect (with cancelled guard where the deps can change) or an effect-local async runner, keeping every setState on the async path; (c) OAuthModal countdown → derived value from deviceCodeExpiresAt + a now tick state; (d) Sidebar localStorage hydration → useSyncExternalStore snapshots + render-time adjustment, persistence consolidated into a single saveToStorage effect
immutability (use-before-declare) 2 PricingModal.loadPricing inlined into its effect; ProxyConfigModal.resetFields hoisted above the effect as a dependency-free useCallback
exhaustive-deps 1 ProxyConfigModal load effect now depends on the stable resetFields and on hoisted translated strings (socks5HiddenError, levelGlobalLabel) instead of the t function identity
preserve-manual-memoization 1 UsageStats.sortedAccounts optional chains destructured into locals so the memo body matches its deps

Per-file

File Violations Technique
KiroAuthModal.tsx set-state-in-effect ×1 close-reset → render adjustment
ModelSelectModal.tsx set-state-in-effect ×4 3 fetch fns moved into their effects; close-reset → render adjustment
OAuthModal.tsx set-state-in-effect ×4 countdown derived (state deleted); provider-change/close/open resets → render adjustments with ref invalidation split into ref-only effects; flow start via effect-local async runner (kept the startsInGitlabDuoSetup / if (!startsInPasteMode) startOAuthFlow() shape the #8688 source-pinning test asserts)
PricingModal.tsx immutability ×1 loadPricing inlined into the effect
ProxyConfigModal.tsx set-state-in-effect ×1, immutability ×1, exhaustive-deps ×1 open-reset → render adjustment; resetFields hoisted stable; deps on hoisted strings (an unstable t, e.g. the test mock, would loop the load effect)
ReasoningRoutingRules.tsx set-state-in-effect ×1 effect-local async runner around load()
RequestLoggerDetail.sections.tsx set-state-in-effect ×1 liveDetail mirror → render adjustment
Sidebar.tsx set-state-in-effect ×2 hydration via useSyncExternalStore + render adjustment (ref → state); active-section expansion → render adjustment keyed on the old effect deps; one persistence effect (also removes a pre-existing eslint-disable for exhaustive-deps)
UsageStats.tsx preserve-manual-memoization ×1, set-state-in-effect ×1 memo locals; mount fetch via effect-local runner (redundant sync setLoading(true) dropped — loading starts true)
analytics/useProviderDailyUsage.ts set-state-in-effect ×1 fetch inlined into the effect with a cancelled guard
compression/ComboCompressionModeSelect.tsx set-state-in-effect ×1 initialCompressionMode mirror → render adjustment

Validation

  • npx eslint --suppressions-location config/quality/eslint-suppressions.json --pass-on-unpruned-suppressions --max-warnings 0 <11 files> → exit 0
  • npm run typecheck:core → clean
  • Node native sweep (54 matching tests/unit/**/*.test.ts files, env -u OMNIROUTE_API_KEY): 373/373 pass (includes the OAuthModal/gitlab-duo source-pinning tests)
  • Vitest sweep (78 matching *.test.tsx files, react18-json-view@0.2.10 installed --no-save): 547/550; the 3 fails are 5s-timeout flakes under parallel load — all 3 files pass isolated (8/8), and one (radar-admin-sidebar) is not a touched file
  • Prettier run on every touched file + the suppressions JSON

Refs #12146

…lations in shared/components

Real refactors (no suppressions, no eslint-disable) for the 21 react-hooks/*
violations across the 11 src/shared/components files of this batch:

- set-state-in-effect (prop/state mirror or modal open/close reset):
  replaced with guarded render-time adjustments (react.dev "You Might Not
  Need an Effect" prev-tracking pattern) — KiroAuthModal,
  ModelSelectModal, ProxyConfigModal, OAuthModal (provider-change, close
  and open resets; ref invalidation split into ref-only effects),
  RequestLoggerDetail.sections (liveDetail mirror),
  ComboCompressionModeSelect (initialCompressionMode mirror).
- set-state-in-effect (fetch+set effects calling component-scope
  functions): moved the async loader inside the effect (ModelSelectModal
  fetchCombos/fetchProviderNodes/fetchCustomModels, PricingModal
  loadPricing, useProviderDailyUsage fetchRows — now with a cancelled
  guard) or wrapped the call in an effect-local async runner
  (ReasoningRoutingRules load, UsageStats fetchStats, OAuthModal
  startOAuthFlow) with every setState on the async path.
- OAuthModal device-code countdown: deviceCodeSecondsRemaining state
  deleted and derived from deviceCodeExpiresAt plus a `now` tick state
  updated by the interval (re-anchored when polling starts).
- Sidebar localStorage hydration: reads moved into useSyncExternalStore
  snapshots (server snapshot null) applied via render-time adjustment;
  skipInitialActiveExpansion ref converted to state; the active-section
  expansion effect became a render-time adjustment keyed on the old
  effect deps; persistence consolidated into one saveToStorage effect
  (removes the saves that ran inside setState updaters and drops a
  pre-existing eslint-disable for exhaustive-deps).
- immutability (use-before-declare): PricingModal loadPricing inlined
  into its effect; ProxyConfigModal resetFields hoisted above the load
  effect as a dependency-free useCallback.
- exhaustive-deps (ProxyConfigModal): effect now depends on the stable
  resetFields and on hoisted translated strings (socks5HiddenError,
  levelGlobalLabel) instead of the `t` identity.
- preserve-manual-memoization (UsageStats sortedAccounts): optional
  chains destructured into locals so the memo deps match the usage.

config/quality/eslint-suppressions.json: removed every react-hooks/*
entry for the 11 files (other-rule entries preserved).

Validation: eslint gate (--suppressions-location, --max-warnings 0) green
on all 11 files; typecheck:core clean; node unit sweep 373/373; vitest
sweep 547/550 with the 3 fails being 5s-timeout flakes under parallel
load (all pass isolated 8/8, one in an untouched file).

Refs #12146
The test (merged with the DuckDuckGo cooldown fix) covers accountFallback.ts and
auth.ts but was not listed, so check:mutation-test-coverage --strict reds any PR
whose merge ref includes it. Base also merged in.
@diegosouzapw
diegosouzapw merged commit bbbcc79 into release/v3.8.51 Aug 31, 2026
21 checks passed
@diegosouzapw
diegosouzapw deleted the chore/12146-batch4-shared-components branch August 31, 2026 02:06
xiaoyaner0201 added a commit to xiaoyaner0201/OmniRoute that referenced this pull request Aug 31, 2026
…51 @ 718accb

Upstream advanced from 7f49b34 to 718accb while R4 was running (two
chore commits: stryker test registration diegosouzapw#12170 and shared/components
react-hooks lint batch diegosouzapw#12159). Neither touches this change's two paths, and
the ESLint suppressions entry for src/app/api/v1/models/catalog.ts is
unchanged at no-unused-vars: 10.

Ordinary additive two-parent merge so the candidate is bound to the exact
current target SHA.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…hooks violations resolved (diegosouzapw#12159)

* chore(lint): batch 4 of diegosouzapw#12146 — resolve the react-hooks compiler violations in shared/components

Real refactors (no suppressions, no eslint-disable) for the 21 react-hooks/*
violations across the 11 src/shared/components files of this batch:

- set-state-in-effect (prop/state mirror or modal open/close reset):
  replaced with guarded render-time adjustments (react.dev "You Might Not
  Need an Effect" prev-tracking pattern) — KiroAuthModal,
  ModelSelectModal, ProxyConfigModal, OAuthModal (provider-change, close
  and open resets; ref invalidation split into ref-only effects),
  RequestLoggerDetail.sections (liveDetail mirror),
  ComboCompressionModeSelect (initialCompressionMode mirror).
- set-state-in-effect (fetch+set effects calling component-scope
  functions): moved the async loader inside the effect (ModelSelectModal
  fetchCombos/fetchProviderNodes/fetchCustomModels, PricingModal
  loadPricing, useProviderDailyUsage fetchRows — now with a cancelled
  guard) or wrapped the call in an effect-local async runner
  (ReasoningRoutingRules load, UsageStats fetchStats, OAuthModal
  startOAuthFlow) with every setState on the async path.
- OAuthModal device-code countdown: deviceCodeSecondsRemaining state
  deleted and derived from deviceCodeExpiresAt plus a `now` tick state
  updated by the interval (re-anchored when polling starts).
- Sidebar localStorage hydration: reads moved into useSyncExternalStore
  snapshots (server snapshot null) applied via render-time adjustment;
  skipInitialActiveExpansion ref converted to state; the active-section
  expansion effect became a render-time adjustment keyed on the old
  effect deps; persistence consolidated into one saveToStorage effect
  (removes the saves that ran inside setState updaters and drops a
  pre-existing eslint-disable for exhaustive-deps).
- immutability (use-before-declare): PricingModal loadPricing inlined
  into its effect; ProxyConfigModal resetFields hoisted above the load
  effect as a dependency-free useCallback.
- exhaustive-deps (ProxyConfigModal): effect now depends on the stable
  resetFields and on hoisted translated strings (socks5HiddenError,
  levelGlobalLabel) instead of the `t` identity.
- preserve-manual-memoization (UsageStats sortedAccounts): optional
  chains destructured into locals so the memo deps match the usage.

config/quality/eslint-suppressions.json: removed every react-hooks/*
entry for the 11 files (other-rule entries preserved).

Validation: eslint gate (--suppressions-location, --max-warnings 0) green
on all 11 files; typecheck:core clean; node unit sweep 373/373; vitest
sweep 547/550 with the 3 fails being 5s-timeout flakes under parallel
load (all pass isolated 8/8, one in an untouched file).

Refs diegosouzapw#12146

* test(mutation): register search-432-plan-limit-cooldown in tap.testFiles

The test (merged with the DuckDuckGo cooldown fix) covers accountFallback.ts and
auth.ts but was not listed, so check:mutation-test-coverage --strict reds any PR
whose merge ref includes it. Base also merged in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant